Skip to content

fix(videoout): report GPU driver shutdown failure from the present thread at Shutdown - #1225

Merged
adriadam10 merged 5 commits into
boykopovar:mainfrom
Pedroj-64:fix/video-out-present-shutdown-failure
Oct 9, 2026
Merged

adriadam10 merged 5 commits into
boykopovar:mainfrom
Pedroj-64:fix/video-out-present-shutdown-failure

Conversation

@Pedroj-64

@Pedroj-64 Pedroj-64 commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

What

Part of #1040.

VideoOutDriver::presentLoop called AgcDriverShutdown_nid_postfix() outside its try block. When the driver had a stored worker failure, Driver::Shutdown rethrew it on the present thread and the process terminated. The failure is now stored in flipQueue->failure, so VideoOutDriver::Shutdown rethrows it after the join, as the lifetime test expects.

video_out_flip_tests is now built with BUILD_TESTING and the lifetime, decode and controls modes are registered in ctest with SDL_VIDEODRIVER=offscreen.

Not included: the reopen mode. There Fail sets the queue-wide failure while the present loop runs and the loop terminates the process (0d29a0a). Should a flip on a closed port still kill the process, or stay a per-port failure as the test expects? The other six modes need a param.json fixture.

Tested

Linux, SDL offscreen, lavapipe: video_out_flip_lifetime, video_out_flip_decode and video_out_flip_controls pass. Without the VideoOutDriver.cpp change video_out_flip_lifetime aborts. tools/check_conventions.py --base upstream/main passes. Not tested on Windows (availability of the offscreen SDL driver there is unverified).

Checklist

  • Based on current main; no other open PR implemented the same functions when this was opened (fix(libSceVideoOut): propagate shutdown driver error and register flip tests #1575 was opened later and overlaps)
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw; no silent stubs added
  • No notes, investigation .md files or images added to the repository
  • New third-party code is a submodule built from source (none added)
  • Generic behaviour, not specific to one title
  • Depends on: none
  • AI-assisted: yes

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Two things before this can go in:

  • On Windows the three video_out_flip_* tests exit with 0xc0000135 (DLL not found). The other tests in core/libs/prx/libSceVideoOut/CMakeLists.txt add ENVIRONMENT_MODIFICATION "PATH=path_list_prepend:$<TARGET_FILE_DIR:libc>" under if(WIN32); these need the same.
  • The conventions check wants the AI-assisted line answered with yes or no.

@popcoder565

popcoder565 commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Corrected. The first version of this comment was wrong. It said lifetime still terminates on Windows with this PR, and blamed the unwind between the two prx modules. My build had stopped before libSceVideoOut.prx was relinked: I had piped cmake --build into a command that stops reading early, so those runs and the gdb sessions used main's driver. Clean results:

Tested on Windows 10 (19045), MinGW-w64 GCC 15.2.0 (WinLibs r7), Release, RTX 3060 Laptop (driver 616.92), merged onto main 73ac9ef:

  • As they are, the three ctest entries exit with 0xc0000135, as noted above.
  • I added ENVIRONMENT_MODIFICATION "PATH=path_list_prepend:$<TARGET_FILE_DIR:libc>" under if(WIN32), as the other tests in this CMakeLists.txt do. With that, all three pass in each of 3 rounds.
  • Running the binary directly with SDL_VIDEODRIVER=offscreen:
    • lifetime passes 6 of 6 with this change.
    • With main's VideoOutDriver.cpp, lifetime terminates 3 of 3 with VideoOut: port is closed.
    • decode and controls pass 3 of 3, so the offscreen driver works on Windows.
  • On Windows, Driver::run also catches the port is closed throw from FlipRequest::GpuReady on the queue worker; I traced the unwinder's search phase. What terminated on main is the shutdown rethrow this PR catches, as video_out_flip_tests is not in ctest and its lifetime and reopen modes terminate on main #1040 describes for Linux.

Tested with AI assistance (Claude Code).

@Pedroj-64

Copy link
Copy Markdown
Contributor Author

Thanks for the review, and to @popcoder565 for testing on Windows.

@LeandroLP23

Copy link
Copy Markdown
Contributor

Windows data for the closed-port question, from a real title. ps5-doom (lowbit/ps5-doom v1.1.0, PPSA99666, GPL homebrew) ends a -timedemo run through I_Error → sceSystemServiceLoadExec("exit", NULL), and the process then terminates on the present thread:

[debug:0] Error: timed 3836 gametics in 2278 realtics
[debug:0] ps5: exit 1
[videoout] presentation failed: VideoOut: port is closed
terminate called after throwing an instance of 'std::runtime_error'
  what():  VideoOut: port is closed

Exit code 0xc0000409, on main 3c0756e and with this PR merged into 63c57d2 (2/2 runs each), so as the description says this path isn't covered here. The same title leaving through the same LoadExec("exit") with code 0, when ps5-doom's frame limit ends a run, did not hit it in 6 runs.

Repro: relink ps5-doom's eboot.bin, then run doom.exe -timedemo demo2 from the app layout. Windows 11, Radeon 860M, MinGW-w64 GCC 15.2.0, Release.

@Pedroj-64

Copy link
Copy Markdown
Contributor Author

Thanks @LeandroLP23 for the ps5-doom data. That exit path is a closed port with a flip still queued, not the shutdown rethrow this PR fixes, and handling it widens the scope here, so it is in a separate PR: #1648. It cancels flips on a port the title closed instead of terminating, and builds on this one. It is untested with ps5-doom itself; if you can run -timedemo with #1648 merged on top of this, that would confirm it.

@MarcZX10

MarcZX10 commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

I checked the failed run for head 2f6c0998 (run 37787482615). Both Linux and Windows stop at Check that every test executable is registered with ctest; the C++ build and tests are skipped. The failure is that this PR registers video_out_flip_tests but leaves it in tools/check_test_registration.py's KNOWN_UNRUN set (error: video_out_flip_tests is run by ctest now; remove it from KNOWN_UNRUN). PR #1575 already includes that removal, so please coordinate the overlap and ensure the selected PR carries it, then rerun CI.

@Pedroj-64

Copy link
Copy Markdown
Contributor Author

Thanks @MarcZX100 for tracing the CI failure. Fixed in 3a64184: video_out_flip_tests is removed from KNOWN_UNRUN in tools/check_test_registration.py, and the branch is merged with current main. tools/check_conventions.py --base upstream/main passes locally; CI is re-running.

On the overlap with #1575: this PR was opened first (2026-10-07) and now carries the same registration fix, so I leave the choice to the maintainers. I will adjust this one if they prefer #1575.

@popcoder565

Copy link
Copy Markdown
Contributor

Update to my Windows results: main now includes #1367, which breaks the offscreen ctest entries on Windows.

#1367 creates the VideoOut window with SDL_WINDOW_VULKAN on the first VideoOut call. SDL's offscreen driver can't create a Vulkan window on Windows, so the tests that open a port now stop at:

DisplayWindow: Vulkan support is either not configured in SDL or not available in current SDL video driver (offscreen) or platform

Setup: Windows 10 (19045), RTX 3060 Laptop (616.92), GCC 15.2.0 (WinLibs r7), Release. I tested #1648, which carries this PR's commits, merged onto main 6cc0503. I also merged #2093, because main doesn't build without it at the moment.

lifetime reopen decode controls
ctest, as registered (SDL_VIDEODRIVER=offscreen) 0/10 0/10 10/10 0/10
same binary with SDL's default Windows driver 3/3 3/3 – 3/3
ctest with #1367 reverted 3/3 3/3 3/3 3/3

So the driver changes in this PR and #1648 work on Windows; only the offscreen registration fails since #1367. My earlier offscreen passes were on 73ac9ef, before #1367 merged. #1575 registers the same tests with SDL_VIDEODRIVER=offscreen, so it's affected the same way.

How to handle it is your call. The AGC device tests return 77 (skipped) when they can't get a device. Dropping the offscreen driver on Windows works on a desktop, but the Windows CI runner has no Vulkan device.

🤖 Addressed by Claude Code

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Please rebase onto main and force-push instead of merging main in (eb899d0). Since #1367 the tests this registers with SDL_VIDEODRIVER=offscreen fail on Windows (popcoder565's results above); they need to pass there, or skip when no Vulkan window can be made, the way the AGC device tests skip without a device.

@MarcZX10

MarcZX10 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

For the Windows offscreen failure, current main already has a matching pattern in GuestVideoOutWqhdDetection.cpp / libSceVideoOut/CMakeLists.txt: catch the Vulkan/display initialization failure, print a skip reason, return 77, and set SKIP_RETURN_CODE 77 on the CTest. That preserves the Linux offscreen coverage and marks the Windows no-Vulkan case as skipped instead of failed. The new flip modes that do not open a Vulkan window can remain runnable on Windows.

Pedroj-64 and others added 3 commits October 9, 2026 06:53
…read at Shutdown

presentLoop called AgcDriverShutdown outside its try block, so a stored
worker failure rethrown there terminated the process. Store it in
flipQueue->failure so VideoOutDriver::Shutdown rethrows it after the join.

Register the lifetime, decode and controls modes of video_out_flip_tests
in ctest.
…Windows

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@Pedroj-64
Pedroj-64 force-pushed the fix/video-out-present-shutdown-failure branch from 3a64184 to ab5803a Compare October 9, 2026 11:55
…ram.json

The offscreen SDL driver cannot create a Vulkan window on Windows since
the VideoOut window uses SDL_WINDOW_VULKAN, so the tests that open a port
now exit 77 (skipped) like the other display tests, unless
ANYPS5_REQUIRE_DISPLAY is set. The tests also write app0/sce_sys/param.json,
which AppMetadata now requires.
@Pedroj-64
Pedroj-64 force-pushed the fix/video-out-present-shutdown-failure branch from ab5803a to 2663699 Compare October 9, 2026 11:56
@Pedroj-64

Copy link
Copy Markdown
Contributor Author

Rebased onto main and force-pushed, no merge commit (2663699). The tests that open a port now exit 77 (skipped) when no Vulkan window can be created, as the AGC device tests do, and Flip.cpp writes app0/sce_sys/param.json, which AppMetadata now requires. Thanks @popcoder565 and @MarcZX100 for the Windows data and the pattern.

@Pedroj-64

Copy link
Copy Markdown
Contributor Author

Lowered the video_out_flip_* CTest timeout from 60 to 30 s to follow CONTRIBUTING.md (59c98bd). On Linux without a Vulkan window decode takes 0.01 s and the other modes skip with 77; real-window durations are pending, see the discussion on #1648.

@krishalivaibhav

Copy link
Copy Markdown
Contributor

Native Apple M2 validation confirms the shutdown behavior addressed by #1225 and the closed-port cancellation follow-up #1648.

Environment: Apple M2 / 16 GB, macOS 27.2 beta 3, Apple Clang 21 / SDK 27, bundled SDL compiled from source, MoltenVK 1.4.2. The local integration already included ARM64/macOS branch 11db79ae, placeholder-image #2015 (17713d1c), SPIR-V 1.3 #2191 (5463ea47), and visual-demo #2244 (1c71428f) on main d70b8998. These are combined-branch results, not either PR alone.

The existing video_out_flip_tests modes ran separately with 30-second timeouts, forced Apple M2 selection, required Vulkan/display initialization, and shader disk cache disabled. We used native Cocoa rather than SDL offscreen. The isolated working directory used the synthetic param.json structure from the existing WQHD fixture; an initial missing-metadata setup failure was corrected without changing production code.

An independent subagent reran pending-close, reopen, controls and presentation successfully, and reviewed error preservation and the macOS conflict resolution. No new production fix was authored. No games, full suite, or visible-output checks were performed; presentation results establish flip state and pacing, not pixel correctness. AI-assisted validation using Codex.

@Pedroj-64

Pedroj-64 commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor Author

Thanks @krishalivaibhav for the Apple M2 validation: it confirms that lifetime aborts without the change and passes with it, and that controls and presentation still work. Thanks also to @popcoder565, @rmz-24 and @MarcZX100 for the Windows data and the reviews; I have no pending changes on this PR.

@adriadam10

Copy link
Copy Markdown
Collaborator

Checked: build + full ctest on current main merged (551/551). video_out_flip_decode passes; lifetime and controls skip with 77 here (no Vulkan window with SDL offscreen since #1367), and the fix is confirmed by @krishalivaibhav on Apple M2 (lifetime aborts without it, passes with it). The catch (...) stores into flipQueue->failure and Shutdown rethrows it, so nothing is swallowed. Note that CI will skip lifetime/controls too for the same reason, so only a real window covers the fix. Merging once CI is green.

@adriadam10
adriadam10 merged commit 58706dc into boykopovar:main Oct 9, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants